Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

Conversation

@vinzenz
Copy link

@vinzenz vinzenz commented Nov 3, 2016

No description provided.

dsnet and others added 2 commits November 3, 2016 10:38
An upstream change (http://golang.org/cl/31144) alters the time package
to be more consistent about the representation of UTC location.
We adjust the time.Time used in the test to work on both 1.7 and 1.8.
@niemeyer
Copy link
Contributor

It's not clear to me how this is fixing the referenced issue in a proper way. What's the actual specification of the float format in yaml, and how is this handling it? Where are the tests for it showing details of the problem being fixed?

@vinzenz
Copy link
Author

vinzenz commented Jan 25, 2017

It's not clear to me how this is fixing the referenced issue in a proper way.

ParseFloat is behaving oddly by discarding the leading 0 and make the rest of the number a float.

What's the actual specification of the float format in yaml, and how is this handling it?

http://yaml.org/spec/1.2/spec.html#tag/repository/float
A leading 0 is only allowed when followed by a dot, which it checks for and then passes the string on to really try a float. (tryFloat)

Where are the tests for it showing details of the problem being fixed?

I will add some test that would fail before

@niemeyer
Copy link
Contributor

Note that this changed from yaml 1.1 to 1.2. It might be okay, but we may also end up breaking existing code. In Python:

>>> import yaml
>>> yaml.load("01.1")
1.1

Thanks for the tests.

@niemeyer
Copy link
Contributor

Curiously, Python float resolution seems broken in other ways:

>>> yaml.load("-2E+05")
'-2E+05'

@vinzenz
Copy link
Author

vinzenz commented Jan 25, 2017

#171 seems to actually fix this already

@vinzenz
Copy link
Author

vinzenz commented Jan 25, 2017

And we can also add this as a parser option (Strict floats) like it was introduced as another PR

@niemeyer
Copy link
Contributor

Indeed, merged #171. Thanks for your help on this.

@niemeyer niemeyer closed this Jan 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants